Skip to content

Fix a few parser style issues. #2442

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

zherczeg
Copy link
Member

No description provided.

Copy link
Contributor

@LaszloLango LaszloLango left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@rerobika rerobika left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (informally)

@zherczeg
Copy link
Member Author

zherczeg commented Aug 1, 2018

The issue is that lexer_hex_to_character is used by some internal tests. I don't really like this, but we can keep it if needed.

@zherczeg
Copy link
Member Author

zherczeg commented Aug 3, 2018

Any suggestion?

@LaszloLango
Copy link
Contributor

Let's keep it or declare as an extern function in the related unit test file.

@akosthekiss
Copy link
Member

My two cents: keep lexer_hex_to_character as non-static. There are quite some other non-API functions that are accessed from unit tests, too. (Even in test-lit-char-helpers.c, e.g., lit_char_get_utf8_length.) For now, we have to live with keeping them non-static, IMHO.

Some (distant?) future work may deal with solving this, i.e., how to make those functions static that are only published for testing reasons. But that will deal with all such functions then, not only this one.

JerryScript-DCO-1.0-Signed-off-by: Zoltan Herczeg [email protected]
@zherczeg zherczeg force-pushed the parser_style_fixes branch from c9df788 to ae57d20 Compare August 6, 2018 06:16
Copy link
Member

@akosthekiss akosthekiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@akosthekiss akosthekiss added the style Related to coding style label Aug 6, 2018
@LaszloLango LaszloLango merged commit 1ac2903 into jerryscript-project:master Aug 6, 2018
@zherczeg zherczeg deleted the parser_style_fixes branch August 10, 2018 06:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
style Related to coding style
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants